Update amp-twitter to use fixed-height layout with a default height matching minimal tweet height #6504
Conversation
…atching minimal tweet height
|
Plugin builds for 4700260 are ready 🛎️!
|
| @@ -161,9 +181,9 @@ private function create_amp_twitter_and_replace_node( Document $dom, DOMElement | |||
| } | |||
|
|
|||
| $attributes = [ | |||
| 'width' => $this->DEFAULT_WIDTH, | |||
| 'width' => 'auto', | |||
There was a problem hiding this comment.
Any reason for this change?
| 'width' => 'auto', | |
| 'width' => $this->DEFAULT_WIDTH, |
There was a problem hiding this comment.
Yes, because auto is the only valid width for the fixed-height layout.
There was a problem hiding this comment.
Oh wait, you're right. I've revised in #6510
|
QA In Progress Question: how should this be QA'ed? Should Bento be enabled and if yes, then which sandboxing level should it be on? Should I use something like this in a Custom HTML block: <script type="module" src="https://cdn.ampproject.org/bento.mjs" crossorigin="anonymous"></script>
<script nomodule="" src="https://cdn.ampproject.org/bento.js" crossorigin="anonymous"></script>
<script type="module" src="https://cdn.ampproject.org/v0/bento-twitter-1.0.mjs" crossorigin="anonymous"></script>
<script nomodule="" src="https://cdn.ampproject.org/v0/bento-twitter-1.0.js" crossorigin="anonymous"></script>
<link rel="stylesheet" href="https://cdn.ampproject.org/v0/bento-twitter-1.0.css" crossorigin="anonymous">
<bento-twitter width="auto" height="197" layout="fixed-height" id="my-tweet-1" data-tweetid="1410342522820300802"></bento-twitter>Or should I just embed a Tweet in the post and let WordPress convert it to an embed? In the latter case, AMP converts the embed to I think that answers to those questions may be important for #6502 and it might need to be QA'ed again. |
|
@delawski Yes, using However, to test this properly you'd want to embed a Tweet using the Tweet embed block. Even if you're in the Loose level for Sandboxing, this alone wouldn't trigger the Bento version since it's a valid AMP page and we currently only opt for Bento versions if there is something on the page that is downgrading it to Moderate or Loose, since Bento components are especially important for encapsulation. So an easy way to force the Moderate level is to just add a Custom HTML block that has this: <script data-px-verified-tag>console.info('PX-verified!');</script>I just checked and in Level 3 with non-Bento component, the markup is: <amp-twitter width="auto" height="197" layout="fixed-height" data-tweetid="1410342522820300802" class="twitter-tweet i-amphtml-layout-fixed-height i-amphtml-layout-size-defined" data-width="550" data-dnt="true" style="height:197px" i-amphtml-layout="fixed-height"><button overflow type="button">See more</button><blockquote class="twitter-tweet" data-width="550" data-dnt="true" placeholder=""><p lang="en" dir="ltr">Rideshare deployment sequence complete</p>— SpaceX (@SpaceX) <a href="https://twitter.com/SpaceX/status/1410342522820300802?ref_src=twsrc%5Etfw">June 30, 2021</a></blockquote></amp-twitter>And with in Level 3 with the Bento component the markup is (also): <amp-twitter width="auto" height="197" layout="fixed-height" data-tweetid="1410342522820300802" class="twitter-tweet i-amphtml-layout-fixed-height i-amphtml-layout-size-defined" data-width="550" data-dnt="true" style="height:197px" i-amphtml-layout="fixed-height"><button overflow type="button">See more</button><blockquote class="twitter-tweet" data-width="550" data-dnt="true" placeholder=""><p lang="en" dir="ltr">Rideshare deployment sequence complete</p>— SpaceX (@SpaceX) <a href="https://twitter.com/SpaceX/status/1410342522820300802?ref_src=twsrc%5Etfw">June 30, 2021</a></blockquote></amp-twitter>The difference is in the rendering:
In non-Bento, there is a layout shift and the entire Tweet is shown. In Bento, there is no layout shift and an overflow button is shown. |
|
QA Passed ✅ After following @westonruter's instructions (thank you for that 🙌), I was able to confirm that Bento version of
|
|
Note also that |
westonruter
added
the
Changelogged
Summary
This is a follow-up on #6502.
When the Bento version of
amp-twitteris embedded on a page, it does not resize automatically when in the initial viewport. Since the default dimensions of an embedded Tweet was 600x480 with responsive layout, the result is an element with a lot of bottom padding. And since the element is taller than the contents require, AMP does not show theoverflowbutton since the element needs to shrink not grow. Therefore, I've updated the default height to match the smallest Tweet I could find. This will ensure that the Tweet will only ever grow in height, and not need to shrink. Also, I changed thelayoutfromresponsivetofixed-heightwhich seems more appropriate, as Tweets generally do have a fixed height.Checklist